Repository navigation
sleep: stop the HR-only fallback inheriting an exclusion written to keep WHOOP out - #1813
Merged
Merged
Conversation
…eep WHOOP out The spine has never once run. The gate line added in #1812 said so on the first build that carried it: [sleep] hr-only gate attempted=false reason=imported-owner grav=0 stored=0 on a day holding 165,980 HR rows. Not a band too tight, not a duration gate - the branch was skipped before any heart rate was looked at. #1806 took its condition verbatim from #804 Fix A, whose comment says it exists "so WHOOP straps and the my-whoop import namespace are untouched". #804 is about Oura RINGS and deliberately excludes WHOOP. The HR-only fallback is FOR a WHOOP strap, so it inherited a condition whose entire purpose was to exclude the case it was built for. resolveDayOwner returns importedDeviceId whenever the owner source is absent or the candidates collapse to it, which on this install is every day - so it was never going to fire, and the p10 anchor I spent a pass tuning was never reached. Split by question rather than by gate. #804's stored-hypnogram branch keeps the owner exclusion, a ring's own night still not coming from the import namespace. The heart-rate fallback drops it. Between them, a new branch: a day that HAS stored sessions is left alone whoever owns it, because inferring a night from heart rate when the device recorded a real one replaces better evidence with worse. The stored lookup now runs for EVERY no-motion day, so "nothing else knows about this night" is checked rather than assumed. That is a new query per no-motion day for the import namespace, which previously skipped it - one indexed LIMIT read, and it is the read that keeps this from inventing nights for import users who already have real ones. Not free, and worth the correctness. Pinned by asserting the shape: the HR-only branch must not mention importedDeviceId, and the stored branch must. A behavioural test cannot reach a gate that needs a repository, and the failure mode is someone reinstating the exclusion for symmetry. Both platforms. 26 HR-only tests, 0 failures. Refs #1801.
…fallback Re-review. Removing the owner exclusion does not only unblock the 5/MG it was written for - it makes the branch reachable for ANY owner, including a WHOOP 4.0 day that banked nothing at all. I had told the maintainer a 4.0 was untouched "because grav.size < 2 is false", which is true of a normal 4.0 day and not true of an empty one. Low risk, and not zero: such a day has too little of anything to clear minSleepMin, but "too little" is not "none", and the night it could produce is display-only and marked hrOnly like every other. The comment now says so at the branch rather than leaving the next reader to work out that the gate widened. Also verified this pass, and sound: the guard test splits the expression on "else ->", and there is exactly one of those, one importedDeviceId, none after the split. The middle branch returning emptyList() for an import-namespace day with stored sessions preserves what both #804 and pre-#1806 did - that path has always yielded nothing there, so nothing moved. And a normal WHOOP day never reaches the stored lookup at all, since grav.size >= 2 short-circuits it, so the added query lands only on days that were already scoring blank.
ryanbr
added a commit
that referenced
this pull request
Sep 2, 2026
Build numbers only: versionCode 413 -> 414, CURRENT_PROJECT_VERSION 294 -> 295. MARKETING_VERSION and versionName stay 11.1.0, which has only ever been cut as a testing build and never released, so the artefacts stay v11.1.0 and 414 simply beats the staging high-water. Carries the gate fix (#1813). Build 413's log showed the HR-only spine had never run at all - attempted=false reason=imported-owner - so this is the first build where it can be reached. Its iOS and macOS legs are also the first compile of the Apple half, which no default CI job builds.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #1801. The spine has never once run.
What the instrumentation found
The gate line from #1812 answered it on the first build that carried it:
165,980 HR rows for the day, and the branch was skipped before any heart rate was looked at. Not
a band too tight, not the duration gate — the two failures the funnel was built to separate. It never
got that far.
The bug, and it is mine
#1806 took its condition verbatim from #804 Fix A, whose comment states it exists "so WHOOP
straps and the
my-whoopimport namespace are untouched". #804 is about Oura rings anddeliberately excludes WHOOP.
The HR-only fallback is for a WHOOP strap. It inherited a condition whose entire purpose was to
exclude the case it was built for.
resolveDayOwnerreturnsimportedDeviceIdwhenever the owner source is absent or the candidatescollapse to it — which on this install is every day. So it was never going to fire, and the p10 anchor
I spent a review pass tuning in #1806 was never reached. That tuning may still have been right; there
is simply no evidence either way yet.
Worth noting the two owner concepts disagree, which is why older logs could not show this: the
[universal] dayOwnerline reportsreadId=whoop-MGB…whileresolveDayOwnerreturnsmy-whoop.The fix — split by question, not by gate
The stored lookup now runs for every no-motion day, so "nothing else knows about this night" is
checked rather than assumed. That is a new query per no-motion day for the import namespace, which
previously skipped it — one indexed
LIMITread. Not free, and it is the read that stops thisinventing nights for import users who already have real ones.
Verification
compileFullDebugKotlin,doc_comment_lint,i18n_audit --ciclean; both platforms restructured identically.
importedDeviceId, and the stored branch must. A source-reading test because the gate needs arepository to reach behaviourally, and because the failure mode is someone reinstating the exclusion
for symmetry.
data is the next question, and sleep: give the HR-only spine a funnel line, because it shipped silent #1812's funnel line is what will answer it.
Re-review: the gate widened for everyone, not just the 5/MG
Dropping the owner exclusion makes the branch reachable for any owner — including a WHOOP 4.0 day
that banked nothing at all (
grav.size < 2, no stored night). I had described a 4.0 as untouched"because
grav.size < 2is false", which is true of a normal 4.0 day and not of an empty one.Low risk and not zero: such a day has too little of anything to clear
minSleepMin, but "too little"is not "none", and any night it produced would be display-only and marked
hrOnly. Now stated at thebranch rather than left for the next reader to derive.
Also verified and sound: the guard test splits on
else ->, and there is exactly one, oneimportedDeviceId, none after the split. The middle branch returning[]for an import-namespace daywith stored sessions preserves what both #804 and pre-#1806 did — that path has always yielded nothing
there. And a normal WHOOP day never reaches the stored lookup at all, since
grav.size >= 2short-circuits it, so the added query lands only on days that were already scoring blank.